P1 — from_join behavioral change is observable on the wire

  The old code combined on and filter as AND(reduce_left(on_conditions), filter). The new code uses conjunction(on_conditions.chain(filter)), which folds right in DataFusion's conjunction
  helper (it uses Iterator::reduce and Expr::and, but the associativity of the resulting tree may differ). Semantically equivalent, but the emitted Substrait byte representation will change
  for joins with 2+ ON predicates plus a filter. Worth:
  - Calling this out in the PR description.
  - Adding a join-with-filter test (or sqllogictest) that exercises both on and filter simultaneously and asserts the produced plan, so future refactors don't silently change wire format
  again.

  P2 — Test coverage is thin

  Only one new test, covering the simplest case (int64 + int64). Given the scope (binary, unary, scalar, higher-order, join refactor, between refactor), consider adding:
  - Unary case (Expr::Not(...) → Boolean, nullable matches input).
  - A nullable-vs-non-nullable case to confirm is_nullable() propagates into the Substrait type's nullability.
  - A from_between test that round-trips both BETWEEN and NOT BETWEEN — the refactor here is non-trivial and lacks direct coverage.
  - A from_join test asserting the join expression structure after the refactor.

  P3 — Expr::to_field(schema)? cost

  Expr::to_field recursively re-derives types from the schema. With this change, every from_binary_expr call now does a to_field over the whole subexpression and recurses into each child,
  which itself re-runs to_field on its subtree. That makes typing O(depth × nodes) over the expression tree. For typical SQL this is unnoticeable, but for generated/synthetic deep
  expressions it can be a quadratic blow-up. Not a blocker, but worth knowing — caching (data_type, nullable) on the way down (or accepting an already-computed type from the caller) would
  fix it.

  P4 — Minor cleanups

  - Expr::ScalarFunction(fun.clone()).to_field(schema)? and Expr::BinaryExpr(expr.clone()).to_field(schema)? clone the expression solely to call a trait method on &Expr. If ExprSchemable
  exposed equivalents on the inner types (ScalarFunction, BinaryExpr), the clones could be avoided. Not introduced by this PR, but the new code makes them more prevalent.
  - In from_between, the *expr.clone() pattern occurs four times in the BETWEEN-not-negated branch:
  Expr::and(
      Expr::lt_eq(*low.clone(), *expr.clone()),
      Expr::lt_eq(*expr.clone(), *high.clone()),
  )
  - A let expr_value = (*expr).clone(); once at the top would tighten this.
  - The new test is #[tokio::test] but the body is fully synchronous — #[test] is sufficient.
  - to_substrait_type(producer, output_field.data_type(), output_field.is_nullable()) is called 3 times. The existing to_substrait_type_from_field would be a slightly cleaner call (after
  wrapping Field → FieldRef). Pure nit.